Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

make ssl listen option configurable #330

Merged
merged 1 commit into from
Jun 11, 2014
Merged

make ssl listen option configurable #330

merged 1 commit into from
Jun 11, 2014

Conversation

saz
Copy link
Sponsor Contributor

@saz saz commented Jun 5, 2014

listen options can only occur once per ip:port combination. Running more than one ssl vhost on the same ip:port combination isn't possible right now.

This PR adds a new parameter 'ssl_listen_option'. With this change, it's possible to disable the ssl listen option on specific vhosts.

@jfryman
Copy link
Contributor

jfryman commented Jun 11, 2014

jfryman pushed a commit that referenced this pull request Jun 11, 2014
make ssl listen option configurable
@jfryman jfryman merged commit 04b78c4 into voxpupuli:master Jun 11, 2014
@3flex
Copy link
Contributor

3flex commented Aug 15, 2014

@saz sorry if I've missed something, but if you don't want a given vhost to listen on SSL, why would you set ssl => true on that vhost resource? When ssl => false the module would not create an SSL enabled vhost in the first place which makes this new option redundant?

Do you have an example problem config that this new parameter addresses?

@saz
Copy link
Sponsor Contributor Author

saz commented Aug 16, 2014

@3flex I want a given vhost to listen on SSL, to be exact, I want multiple vhosts to listen on the same ip:port combination. This wasn't possible before, as every vhost would have ssl in the listen line.

A listen directive can have several additional parameters specific to socket-related system calls. These parameters can be specified in any listen directive, but only once for a given address:port pair.

@3flex
Copy link
Contributor

3flex commented Aug 16, 2014

Hi @saz,

That sentence from the config only applies to certain parameters, but not ssl. You can have a config like this:

server {
  server_name default;
  listen *:80 ssl deferred so_keepalive=on;
  ...
}
server {
  server_name ...;
  listen *:80 ssl;
  ...
}

And that will work fine. But if you include any of the other socket-related options (such as deferred or so_keepalive) in the second server you will get a "duplicate listen options". ssl is safe to include in multiple listen directives. The sentence you quoted only applies to:

  • setfib=number
  • fastopen=number
  • backlog=number
  • rcvbuf=size
  • sndbuf=size
  • accept_filter=filter
  • deferred
  • bind
  • ipv6only=on|off
  • so_keepalive=on|off|[keepidle]:[keepintvl]:[keepcnt]

So the problem I see with the vhost resource right now is if you enable IPv6. You'll get multiple vhosts with ipv6only=on which will cause "duplicate listen options" errors. You'll also have more than one address:port pair with default set which will cause "a duplicate default server" error.

I'm going to open a PR to revert and fix the IPv6 config, feel free to continue discussion there (I'll link from here when I've done it).

@3flex
Copy link
Contributor

3flex commented Aug 16, 2014

See #411.

cegeka-jenkins pushed a commit to cegeka/puppet-nginx that referenced this pull request Oct 23, 2017
make ssl listen option configurable
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants